Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PR: Phoenix LiveView Drag-and-Drop Example #5

Merged
merged 33 commits into from
Nov 3, 2022
Merged

Conversation

SimonLab
Copy link
Member

Create Phoenix application
see: #4

Run the current examples with node
Add lock package file to gitignore
@SimonLab SimonLab added the in-progress An issue or pull request that is being worked on by the assigned person label Oct 19, 2022
@SimonLab SimonLab self-assigned this Oct 19, 2022
- Create new phx app
- Add Tailwind to the phoenix application
@SimonLab
Copy link
Member Author

I've added Tailwind to the Phoenix application with c4f9743

However the hot reload when changing a css class doesn't seem to work. I have to stop and restart the server to be able to see the Tailwind changes. @nelsonic when you have some time, do you mind to try this branch to see if changing a css Tailwind class automatically apply the new style on your browser. I'm not sure if it's a configuration issue on my machine or on the Phx application itself.

@nelsonic
Copy link
Member

@SimonLab remember to add the "watcher". Find the watchers section in config/dev.exs and add:

tailwind: {Tailwind, :install_and_run, [:default, ~w(--watch)]}

See: https://github.com/dwyl/learn-tailwind/blob/61886d3a9821873c02c2bae7cc3a42692b90ef58/config/dev.exs#L17-L21

@SimonLab
Copy link
Member Author

Yes I've added the watcher here:

watchers: [
# Start the esbuild watcher by calling Esbuild.install_and_run(:default, args)
esbuild: {Esbuild, :install_and_run, [:default, ~w(--sourcemap=inline --watch)]},
tailwind: {Tailwind, :install_and_run, [:default, ~w(--watch)]}
]

That's why I'm not sure what the css is not compiled automatically when I changed a class

@nelsonic
Copy link
Member

Hmm ... that's strange. Let me try and run it on my localhost. 👨‍💻
If you want to get on Zoom to remote-pair/debug this LMK (via Signal). 👌

@nelsonic
Copy link
Member

@SimonLab I did:

git checkout 'phoenix-example-#4'
mix deps.get
mix assets.deploy
mix s

Works for me:

tailwind-hot-reloading-working.mov

@SimonLab
Copy link
Member Author

Thanks for checking, I didn't run mix assets.deploy as I thought it was used only on production.
I'll try again now

@SimonLab
Copy link
Member Author

Weirdly hotreload works with Tailwind v3.18, however v3.2.0 or v3.2.1 doesn't change the UI on reload!
@nelsonic did you run mix tailwind.install to make sure you tested with v3.2.0?

I don't want to spend too much time on this specific issue, if it's working with 3.1.8 I'll continue with this and see later on if I can find what is the problem with v3.2

- hotreload doesn't seem to work with 3.2 so using 3.1.8
Add alpine.js cdn in root file
Create button using Alpine to test everything still ok
Create item schema using phx.gen.live
@nelsonic
Copy link
Member

@SimonLab yeah, it worked with Tailwind v3.2.0 but totally fine to downgrade if it allows you to make progress. 👌

Run mix format
Create index when new item created
Update modal form to create item
Update and remove unused tests
@nelsonic
Copy link
Member

@SimonLab looking great so far. Thanks for the demo during standup. 👌

@SimonLab SimonLab changed the title Phoenix example PR: Phoenix example Oct 24, 2022
First attempt at drag and drop with vanilla js version
Drag and drop with javascript
@SimonLab
Copy link
Member Author

SimonLab commented Oct 24, 2022

It took me a bit of time to implement a solution using javascript: eeba5be

My goal is now to see how to implement the vanilla js logic using Alpine.js.
However I've noticed a few things I need to fix:

  • The new item created are not displayed on connected clients, I need to make sure the liveview event is dispatch to all browsers
  • Adding a new item break the js logic. I think it's because the dom as changed with liveview and js is not able to automatically see the new changes. I'll need to see how to work with LiveView and JS. Maybe "Dom patching" could be a solution: https://hexdocs.pm/phoenix_live_view/dom-patching.html

use bg-red-100 to see moving item
Set background color when an element is moved
Use Alpine.js to drag and drop an item in a list
@SimonLab SimonLab self-assigned this Nov 1, 2022
@SimonLab SimonLab added in-progress An issue or pull request that is being worked on by the assigned person and removed awaiting-review An issue or pull request that needs to be reviewed labels Nov 1, 2022
Remove Tailwind and Petal Components to make the setup easier.
Read again the doc and make it easier to follow/implement
@SimonLab
Copy link
Member Author

SimonLab commented Nov 1, 2022

@LuchoTurtle I've updated the documentation (you won't need to install Petal) and this should be easier to setup and follow but let me know if you still spot some issues, thanks

@SimonLab SimonLab removed their assignment Nov 1, 2022
@SimonLab SimonLab added awaiting-review An issue or pull request that needs to be reviewed and removed in-progress An issue or pull request that is being worked on by the assigned person labels Nov 1, 2022
@nelsonic nelsonic added in-review Issue or pull request that is currently being reviewed by the assigned person and removed awaiting-review An issue or pull request that needs to be reviewed labels Nov 2, 2022
@nelsonic
Copy link
Member

nelsonic commented Nov 2, 2022

git checkout 'phoenix-example-#4'
git pull
mix deps.get
mix ecto.reset
mix s

drag-and-drop-demo

Still need to finish looking at the code in more detail. But so far so good. 👌

@SimonLab
Copy link
Member Author

SimonLab commented Nov 2, 2022

I forgot to update the code to the latest version described in the doc. I'm going to do this soon! Thank for testing the PR!

@LuchoTurtle
Copy link
Member

Finished going through the guide and got it working splendidly! Thank you @SimonLab !

I'll leave the code review to the pros 😅

Update Phoenix app code to match the one describe in the markdown
documentation
<h1>Listing Items</h1>

<%= if @live_action in [:new, :edit] do %>
<.modal return_to={Routes.item_index_path(@socket, :index)}>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SimonLab do we need to use a <.modal> for creating new items? 🤷‍♂️

Copy link
Member Author

@SimonLab SimonLab Nov 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The modal is created automatically by Phoenix when running the phx.gen.live command at https://github.com/dwyl/learn-alpine.js/blob/phoenix-example-%234/drag-and-drop.md#create-items

I thought about updating the way Phoenix creates the items however I didn't want to make the tutorial longer and preferred to focus on the drag and drop feature part itself.

Otherwise I can create another documentation file to explain how to create from scratch (without using the phx gen.live command) the liveView structure for creating the items.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SimonLab thanks for clarifying that this was generated by phx.gen.live. 👌
It's definitely concerning [to me] that someone in the Phoenix team 🤷‍♂️
thought it was "OK" to include modals in the default UX for creating new content. 🤦‍♂️
Modals are not even remotely "OK"; they're a UX anti-pattern 😢
and a major red flag of lazy design. 🔴

For the purposes of this example/spike, 🧑‍💻
the objective is to demo drag-and-drop 🐉
and you have achieved that. ✅

In terms of our scarce/valuable time we don't need to spend any more on this. 💸
I've made a note to write about it: dwyl/product-ux-research#38 📝
We will completely avoid them in our App. 🙅

Copy link
Member

@nelsonic nelsonic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SimonLab excellent code and docs!
Would be good to add some tests as the coverage is quite low ...

mix c

Coverage:

----------------
COV    FILE                                        LINES RELEVANT   MISSED
  0.0% lib/app.ex                                      9        0        0
 75.0% lib/app/application.ex                         36        4        1
  0.0% lib/app/repo.ex                                 5        0        0
 61.1% lib/app/tasks.ex                              146       18        7
100.0% lib/app/tasks/item.ex                          18        2        0
  0.0% lib/app_web.ex                                110        1        1
  0.0% lib/app_web/controllers/page_controller.        7        1        1
  0.0% lib/app_web/endpoint.ex                        46        0        0
 41.7% lib/app_web/live/item_live/form_componen       55       12        7
 33.3% lib/app_web/live/item_live/index.ex           107       18       12
  0.0% lib/app_web/live/item_live/show.ex             21        5        5
 90.0% lib/app_web/live/live_helpers.ex               60       10        1
 37.5% lib/app_web/router.ex                          32        8        5
 80.0% lib/app_web/telemetry.ex                       71        5        1
 25.0% lib/app_web/views/error_helpers.ex             30        4        3
100.0% lib/app_web/views/error_view.ex                16        1        0
  0.0% lib/app_web/views/layout_view.ex                7        0        0
  0.0% lib/app_web/views/page_view.ex                  3        0        0
100.0% test/support/conn_case.ex                      38        2        0
 57.1% test/support/data_case.ex                      58        7        3
100.0% test/support/fixtures/tasks_fixtures.ex        20        2        0
[TOTAL]  53.0%
----------------

Some of these files can be ignored, but others will need to have tests. 💭
We can add a chore to the backlog to create these tests. 🙏
and ideally write very clear docs for the tests 📝
because this feature will be "core" to our App. 📱

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in-review Issue or pull request that is currently being reviewed by the assigned person priority-1 Highest priority issue. This is costing us money every minute that passes.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants